Skip to content

[TT-15243] Influx2 latency metric#900

Merged
andrei-tyk merged 2 commits intomasterfrom
influx2-latency-metric
Oct 9, 2025
Merged

[TT-15243] Influx2 latency metric#900
andrei-tyk merged 2 commits intomasterfrom
influx2-latency-metric

Conversation

@andrei-tyk
Copy link
Copy Markdown
Contributor

@andrei-tyk andrei-tyk commented Oct 8, 2025

Description

This PR has been opened to be able to run all release workflows.
Initial external contribution PR: #881

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side). If pulling from your own
    fork, don't request your master!
  • Make sure you are making a pull request against the master branch (left side). Also, you should start
    your branch off our latest master.
  • My change requires a change to the documentation.
    • If you've changed APIs, describe what needs to be updated in the documentation.
  • I have updated the documentation accordingly.
  • Modules and vendor dependencies have been updated; run go mod tidy && go mod vendor
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • Check your code additions will not fail linting checks:
    • go fmt -s
    • go vet

@andrei-tyk andrei-tyk mentioned this pull request Oct 8, 2025
Closed
14 tasks
@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Oct 8, 2025

🔍 Code Analysis Results

Powered by Visor from Probelabs

Last updated: 2025-10-08T19:51:51.412Z | Triggered by: opened | Commit: 9af7ab7

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Oct 8, 2025

🔍 Code Analysis Results

✅ Security Check Passed

No security issues found – changes LGTM.

Performance Issues (1)

Severity Location Issue
🟢 Info pumps/influx2.go:194-213
A new map `mapping` is created for each analytic record inside the loop. For high-throughput scenarios where the `data` slice can contain many records, this leads to frequent memory allocations and increases pressure on the garbage collector, potentially degrading performance.
💡 SuggestionTo optimize memory usage and reduce GC overhead, consider using a pool of maps (e.g., `sync.Pool`) to reuse map allocations. A map can be acquired from the pool at the start of the loop iteration and released back to the pool at the end after it's been used. This is a common pattern for optimizing hot loops that process batches of data.

Quality Issues (1)

Severity Location Issue
🟡 Warning pumps/influx2_test.go:31-48
The test `TestInflux2PumpMappingIncludesLatency` does not test the production code's behavior. Instead of calling the `WriteData` method of `Influx2Pump`, it duplicates the implementation logic for creating the data map. This makes the test brittle and provides a false sense of security, as changes to the implementation in `influx2.go` will not be caught by this test.
💡 SuggestionRefactor the test to be a proper unit test. It should instantiate the `Influx2Pump`, mock its dependencies (like the InfluxDB client), call the `WriteData` method, and then assert that the data passed to the mock client is correct. This will ensure the test is verifying the actual behavior of the component, not just a reimplementation of its logic.

Style Issues (1)

Severity Location Issue
🟡 Warning pumps/influx2_test.go:32-49
The test `TestInflux2PumpMappingIncludesLatency` duplicates the mapping logic from `pumps/influx2.go` instead of testing the production code directly. This makes the test brittle, as changes in the production mapping logic will not be caught by this test, and the test itself will need to be manually kept in sync.
💡 SuggestionRefactor the mapping creation in `pumps/influx2.go` into a separate, testable function. This new function can then be called from both the `WriteData` method and this test, ensuring the test validates the actual production logic and improving maintainability.

Powered by Visor from Probelabs

Last updated: 2025-10-08T19:51:52.338Z | Triggered by: opened | Commit: 9af7ab7

@andrei-tyk andrei-tyk changed the title Influx2 latency metric [TT-15243] Influx2 latency metric Oct 9, 2025
@andrei-tyk andrei-tyk merged commit ae21b3b into master Oct 9, 2025
52 of 55 checks passed
@andrei-tyk andrei-tyk deleted the influx2-latency-metric branch October 9, 2025 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lghiur lghiur lghiur approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@andrei-tyk @lghiur @deeparavirdc